fix: [#1910] Replace custom CSS selector parser with parsel-js#2010
fix: [#1910] Replace custom CSS selector parser with parsel-js#2010TrevorBurnham wants to merge 1 commit into
Conversation
|
Looks like this also fixes #1912. |
…l-js Complex CSS selectors with deeply nested pseudo-classes like :not(:has(> .child:not([attr]))) were being parsed incorrectly. Replaced the custom regex-based parser with parsel-js, a battle-tested CSS selector parser by Lea Verou (~5KB minified) that handles arbitrarily nested parentheses correctly. Added tests for the reported issue.
79ba4ed to
8c749ac
Compare
|
Also fixes #2034. |
|
It seems like this has a noticeable performance impact. Also "parcel-js" has quite few downloads and hasn't been updated for some time. I would prefer to fix the logic in Happy DOM if possible. Each library added needs to be maintained and adds some risk. |
I understand your concerns, but I suspect that any regex-based solution will always have edge-case bugs. I could imagine a hybrid solution where trivial selectors are parsed with regex and those with more complexity are handled by a more robust parser. In the meantime, I've identified some low-hanging optimization fruit in parsel-js and sent over a PR: LeaVerou/parsel#85 Let's see where that goes. |
|
@TrevorBurnham Thank you for your contribution! As mentioned I prefer to make a performant version in Happy DOM. I created #2068 with a fix that uses 2-3 layers of RegExp to separate the groups, but still in an efficient way, which I released now. I've added unit tests for all issues and it also covers the tests in this PR. |
|
I will close this PR now. |
Fixes #1910
Problem
Complex CSS selectors with deeply nested pseudo-classes were being parsed incorrectly, causing wrong query results. For example:
The issue was that the custom regex-based parser only handled one level of nested parentheses.
Solution
Replaced the custom regex-based CSS selector parser with parsel-js, a battle-tested CSS selector parser by Lea Verou.
Benefits:
Implementation Details
The new parser:
\:,\#,\&,\[,\])SelectorItemformatEdge cases handled:
[attr=""])[attr="bracket[]bracket"]):has()(e.g.,:has( +h2))Testing
Added two test cases for issue #1910:
:not(:has(> .child:not([attr])))parsingUpdated tests for stricter CSS compliance:
.before:,.before&after,button:not([type]) now throw errors instead of returning empty resultsChanges
packages/happy-dom/package.json: Addedparsel-jsdependencypackages/happy-dom/src/query-selector/SelectorParser.ts: Rewrote to use parsel-js for parsingpackages/happy-dom/test/query-selector/QuerySelector.test.ts: Added tests for issue Error when parsing complex CSS has expression #1910 and updated edge case tests